-
-
Notifications
You must be signed in to change notification settings - Fork 391
Use an importance score to order the suggested import code action #3271
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use an importance score to order the suggested import code action #3271
Conversation
7727c92
to
fc7d687
Compare
Hi! Please include a test, or an example how the behaviour is now improved! (I've seen how much it is improved, but we want to document it for everyone else as well) |
plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
Outdated
Show resolved
Hide resolved
plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
Outdated
Show resolved
Hide resolved
plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
Outdated
Show resolved
Hide resolved
e30fdb8
to
8aece66
Compare
8aece66
to
f6f1c83
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, awesome!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Were we going to get some tests?
data ImportSuggestion = ImportSuggestion !Int !CodeActionKind !NewImport | ||
deriving ( Eq ) | ||
|
||
instance Ord ImportSuggestion where |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this Ord
instance is not lawful because it doesn't compare the CodeActionKind
, which is taken into account by the Eq
instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching this! I will fix this ASAP
CodeAction
,NewImport
) into anImportSuggestion
ImportSuggestion
first based on the score in descending order and second based on the import module names in alphabetic orderBefore the PR changes:
After the PR changes: